fix: #2883 pickle data was truncated error in database session using MySql #2884
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Lin-Nikaido, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request resolves a critical data truncation issue affecting the storage of pickled objects in MySQL database sessions. By adjusting the data type mapping for MySQL, it ensures that larger serialized data can be correctly persisted and retrieved, preventing unpickling errors and data loss.
Highlights
- Fix for pickle data truncation with MySQL: The
DynamicPickleTypewas causing_pickle.UnpicklingErrorwhen large data was stored in MySQL sessions because it defaulted toBLOB, which has a smaller storage capacity. This change explicitly configuresDynamicPickleTypeto useLONGBLOBfor MySQL dialects, ensuring proper storage and retrieval of large pickled objects.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This change correctly addresses the _pickle.UnpicklingError on MySQL by using LONGBLOB instead of the default BLOB for DynamicPickleType. The implementation is consistent with how other dynamic types like DynamicJSON are handled in this file, and it effectively resolves the data truncation issue.
Regarding your question about testing: to test this database-specific change, you would typically need an integration test that runs against a MySQL database. The existing unit tests in tests/unittests/sessions/test_session_service.py use an in-memory SQLite database, which is why you couldn't find a suitable place.
For a new test, you could:
- Set up a test environment that can connect to a MySQL instance (e.g., using Docker for local testing or a dedicated test DB in CI).
- Create a new test file, perhaps in the
tests/integration/directory. - In that test, initialize
DatabaseSessionServicewith a MySQL connection URL. - Write a test case that creates a session and appends an event with a large
actionspayload that would exceed the size limit ofBLOBbut fit intoLONGBLOB. - Assert that the event can be appended and retrieved successfully without a
UnpicklingErroror data truncation error.
This would ensure the fix works as expected and protects against future regressions. Since setting this up might be outside the scope of this small fix, it could also be handled in a follow-up task.
The change looks good to merge.
adk-bot
added
bot triaged
services
|
Response from ADK Triaging Agent Hello @Lin-Nikaido, thank you for submitting this pull request! To help us review it more effectively, could you please add a Thank you! |
DeanChensj
added
the
ready to pull
…MySql Merge #2884 closes: #2883 # Fix When put leage data into event and load it. the _pickle.UnpicklingError was occurred. The root caurse is `DynamicPickleType` mapping `BLOB` as default in case of MySql, not `LONGBLOB`. And learge data will be able to cut off tail of data. And raise pickle error. # What todo Defined `LONFBLOB` as default explicitly. # Question Where should we code the test code like this case? I cannot found the test code the DB and table was created expectedly. COPYBARA_INTEGRATE_REVIEW=#2884 from Lin-Nikaido:fix/#2883-mysql-datatype-fix 2be9b38 PiperOrigin-RevId: 819891727
|
Merged in 36c96ec |
|
from google.adk.agents import Agent root_agent = Agent( |
…using MySql Merge google#2884 closes: google#2883 # Fix When put leage data into event and load it. the _pickle.UnpicklingError was occurred. The root caurse is `DynamicPickleType` mapping `BLOB` as default in case of MySql, not `LONGBLOB`. And learge data will be able to cut off tail of data. And raise pickle error. # What todo Defined `LONFBLOB` as default explicitly. # Question Where should we code the test code like this case? I cannot found the test code the DB and table was created expectedly. COPYBARA_INTEGRATE_REVIEW=google#2884 from Lin-Nikaido:fix/google#2883-mysql-datatype-fix 2be9b38 PiperOrigin-RevId: 819891727
5 participants
closes: #2883
Fix
When put leage data into event and load it. the _pickle.UnpicklingError was occurred.
The root caurse is
DynamicPickleTypemappingBLOBas default in case of MySql, notLONGBLOB. And learge data will be able to cut off tail of data. And raise pickle error.What todo
Defined
LONFBLOBas default explicitly.Question
Where should we code the test code like this case? I cannot found the test code the DB and table was created expectedly.